Your new package pull request met all of the guidelines for auto-merging and is scheduled to be merged when the mandatory waiting period (3 days) has elapsed.

Since you are registering a new package, please make sure that you have read the package naming guidelines: https://pkgdocs.julialang.org/v1/creating-packages/#Package-naming-guidelines

If you want to prevent this pull request from being auto-merged, simply leave a comment. If you want to post a comment without blocking auto-merging, you must include the text [noblock] in your comment. You can edit blocking comments, adding [noblock] to them in order to unblock auto-merging.

Hi @gerlero and congrats on the package!
Since it is a very small package, have you considered contributing the code to ForwardDiff instead?
PS: This comment is voluntarily blocking the registration but I can make it non blocking afterwards so no worries.

@gdalle Yes, it's very small (at least for now—I may add more features later), but I wanted a standalone package given that I'm using undocumented ForwardDiff functionality (which means it needs its own testing), plus I'm using it already in various projects—including the registered Fronts package for which it'd be a direct dependency.

As for contributing the code here to ForwardDiff, and these were just my own thoughts, but I'm not sure they'll be able to take the changes given the fact that ForwardDiff already offers a different API for single-pass value and derivative via DiffResults, plus this package's current dependency on Unitful (which could be made a weakdep, but I'm not able to work on that for now) which might very likely be a dealbreaker for ForwardDiff. FWIW there are open issues at ForwardDiff for these features @ JuliaDiff/ForwardDiff.jl#328 & JuliaDiff/ForwardDiff.jl#610 where some of this stuff has been discussed.

Is the package still acceptable for registration under these circumstances?

[noblock]

[noblock] Hi there,
great that you are registering a package! I feel that your first argument of using ForwardDiff internals is even an argument more for including your part into ForwardDiff, since now you might just have broken code with a patch release of forward diff, since they would probably consider their internals something nonbreaking to change.

Concerning the second part – you could maybe ask for help concerning the weak dependency and maybe first ask whether it is a dealbreaker for them?
I personally would prefer if you try that first – as well as if this would be within ForwardDiff – but I am not involved in that package and @gdalles comment is already blocking, so I trust his final decision to unblock – hence this is nonblocking :)

@gdalle @kellertuer All right, I'll take the suggestion. As much as I'd prefer a simple wrapper package for this (it's how I've been doing it for a while), I understand your points and will try to open a PR at ForwardDiff for the value_and_derivative(s) functions in the next few days (AFAIC the Unitful compatibility part is nice to have, but for now can wait).

Thank you!
I could definitely see Unitful compatibility as a package extension of either Unitful or ForwardDiff (probably Unitful if I had to pick)

I could definitely see Unitful compatibility as a package extension of either Unitful or ForwardDiff (probably Unitful if I had to pick)

I actually did think about extending Unitful for that, but unfortunately we can't rely on simply adding new methods, as the differentiated function can have units on its output but not on its input.

So, for doing the transparent unit unwrapping with Unitful.ustrip that this package does, one would have to redefine the base ForwardDiff methods depending on whether Unitful is present (not sure if that's possible/easy to do with a weakdep). I thought of other alternatives for stripping units generically such as doing x/oneunit(x), but these are also suboptimal.

EDIT: I ended up finding a way to add Unitful support as a plain extension.

Thanks again for the feedback and suggestions. I've taken the time and effort and have opened the relevant PRs at ForwardDiff. I'm hoping the ForwardDiff maintainers find them acceptable and the changes can be merged there; if that happens then I won't have a reason to pursue this further.

Very cool, thank you!
I don't have much experience with ForwardDiff internals but I'm hoping someone will help review your contributions.

Bump to avoid this being closed (ForwardDiff PRs are still pending review).

Bump

Thanks again for the feedback. I'd like to close this one for now in favor of the changes merged into AbstractDifferentiation.

This pull request has been inactive for 30 days and will be automatically closed 7 days from now. If this pull request should not be closed, please either (1) fix the AutoMerge issues and re-trigger Registrator, which will automatically update the pull request, or (2) post a comment explaining why you would like this pull request to be manually merged. [noblock]